Skip to content

fix: add tmpfiles for faillock dirs #73596

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

fix: add tmpfiles for faillock dirs #73596

wants to merge 3 commits into from
+13 −3

Conversation

@89luca89
Copy link
Member

@89luca89 89luca89 commented Dec 2, 2025

Ref VMS-231

@89luca89 89luca89 requested review from markusboehme and xnox December 2, 2025 15:10
@octo-sts
Copy link
Contributor

octo-sts bot commented Dec 2, 2025

🌐 Build Failed: Environment

chgrp: /home/build/melange-out/linux-pam/bin/unix_chkpwd: No such file or directory

Build Details

Category Details
Build System meson/ninja
Failure Point chgrp command during installation phase

Root Cause Analysis 🔍

The chgrp command is trying to change group ownership of unix_chkpwd at /home/build/melange-out/linux-pam/bin/unix_chkpwd, but this file does not exist at that path. The file was actually installed to /home/build/melange-out/linux-pam/usr/bin/unix_chkpwd according to the installation logs, indicating a path mismatch in the post-installation script or configuration.

🔍 Build failure fix suggestions

Found similar build failures that have been fixed in the past and analyzed them to suggest a fix:

Suggested Changes

File: melange.yaml

  • modification at line around line 50-51 (pipeline runs section)
    Original:
      # make "unix_chkpwd" shadow group and enable g+s
      chgrp shadow ${{targets.destdir}}/bin/unix_chkpwd \
        && chmod g+s ${{targets.destdir}}/bin/unix_chkpwd

Replacement:

      # make "unix_chkpwd" shadow group and enable g+s
      chgrp shadow ${{targets.destdir}}/usr/bin/unix_chkpwd \
        && chmod g+s ${{targets.destdir}}/usr/bin/unix_chkpwd

Content:

Change the path from /bin/unix_chkpwd to /usr/bin/unix_chkpwd to match where meson actually installs the binary
Click to expand fix analysis

Analysis

No similar build failures were provided for analysis. However, analyzing the current failure: The error occurs because the chgrp command is looking for unix_chkpwd at /home/build/melange-out/linux-pam/bin/unix_chkpwd but the file was actually installed to /home/build/melange-out/linux-pam/usr/bin/unix_chkpwd. This is a path mismatch issue where the meson configuration specified -Dsbindir=/usr/bin which affects the installation path, but the post-installation script assumes the binary is in /bin.

Click to expand fix explanation

Explanation

The fix addresses the root cause of the build failure by correcting the path mismatch. The meson build system was configured with -Dsbindir=/usr/bin, which means executables that would normally go to /sbin are instead installed to /usr/bin. The unix_chkpwd binary is one such executable. However, the post-installation script was still trying to access it at the old path /bin/unix_chkpwd. By updating the script to use /usr/bin/unix_chkpwd, we align the script with the actual installation path determined by the meson configuration. This ensures that the chgrp and chmod commands can find the file and properly set its permissions.

Click to expand alternative approaches

Alternative Approaches

  • Remove the -Dsbindir=/usr/bin option from meson configuration to let unix_chkpwd install to its default location, but this would conflict with the merged-usrsbin dependency which expects binaries in /usr/bin
  • Use a find command to locate unix_chkpwd dynamically before applying chgrp/chmod, but this adds unnecessary complexity when the path can be determined from the meson configuration

Was this comment helpful? Please use 👍 or 👎 reactions on this comment.

@octo-sts octo-sts bot added the ai/skip-comment Stop AI from commenting on PR label Dec 2, 2025
@89luca89 89luca89 force-pushed the fix/linux_pam_tmpfiles branch from 87c47f5 to 835018c Compare December 2, 2025 16:07
xnox
xnox reviewed Dec 2, 2025
View reviewed changes
@89luca89
fix: add tmpfiles for faillock dirs
bf9c6c4 
Ref VMS-231

Signed-off-by: Luca Di Maio <[email protected]>
@89luca89
fix: usrmerge fixes
4a9b77e 
Signed-off-by: Luca Di Maio <[email protected]>
@89luca89
fix: disable setuidgid check
a796b4e 
Signed-off-by: Luca Di Maio <[email protected]>
@89luca89 89luca89 force-pushed the fix/linux_pam_tmpfiles branch from f872ef6 to a796b4e Compare December 2, 2025 17:20
@89luca89 89luca89 requested a review from xnox December 2, 2025 17:20
@octo-sts octo-sts bot added the bincapz/pass bincapz/pass Bincapz (aka. malcontent) scan didn't detect any CRITICALs on the scanned packages. label Dec 2, 2025
@egibs
Copy link
Member

egibs commented Dec 2, 2025

The ci-diff-report changes are expected:

usr/bin/unix_chkpwd
+ NOTE: permissions changed for usr/bin/unix_chkpwd: 755 -> 2755
+ WARNING: usr/bin/unix_chkpwd now has special permissions: setgid
- -rwxr-xr-x 91792 2025-10-16 12:19:44 unix_chkpwd
-   APK-TOOLS.checksum.SHA1: "a8b870aa360a77e09f1f922c5e510b4774218d95"
+ grwxr-xr-x 91840 2025-12-02 17:21:44 unix_chkpwd
+   APK-TOOLS.checksum.SHA1: "3441e675a0f35770434a2eda1437a3c8c50bac3a"

due to:

chgrp shadow ${{targets.destdir}}/usr/bin/unix_chkpwd \
  && chmod g+s ${{targets.destdir}}/usr/bin/unix_chkpwd

@89luca89
Copy link
Member Author

89luca89 commented Dec 2, 2025

The ci-diff-report changes are expected:

usr/bin/unix_chkpwd
+ NOTE: permissions changed for usr/bin/unix_chkpwd: 755 -> 2755
+ WARNING: usr/bin/unix_chkpwd now has special permissions: setgid
- -rwxr-xr-x 91792 2025-10-16 12:19:44 unix_chkpwd
-   APK-TOOLS.checksum.SHA1: "a8b870aa360a77e09f1f922c5e510b4774218d95"
+ grwxr-xr-x 91840 2025-12-02 17:21:44 unix_chkpwd
+   APK-TOOLS.checksum.SHA1: "3441e675a0f35770434a2eda1437a3c8c50bac3a"

due to:

chgrp shadow ${{targets.destdir}}/usr/bin/unix_chkpwd \
  && chmod g+s ${{targets.destdir}}/usr/bin/unix_chkpwd

Yea I think before the chgrp whas silently failing I guess

@89luca89 89luca89 added the approved-to-run A repo member has approved this external contribution label Dec 2, 2025
@89luca89 89luca89 requested review from a team December 2, 2025 18:17
@a-crate
Copy link
Member

a-crate commented Dec 2, 2025

Out of curiosity, why is this a tmpfiles.d snippet instead of just mkdir -p ${{targets.destdir}}/var/log/faillock in the package build script?

@sergiodj
Copy link
Member

sergiodj commented Dec 3, 2025

Out of curiosity, why is this a tmpfiles.d snippet instead of just mkdir -p ${{targets.destdir}}/var/log/faillock in the package build script?

I believe it's because of the SELinux context thing, although it should be possible to use mkdir -pZ .... I'll wait for a reply before approving this PR.

@89luca89
Copy link
Member Author

89luca89 commented Dec 3, 2025

mkdir -pZ

That won't work, we need it to be created at runtime to get the right context, else it is created with the var_log one:

With mkdir:

 # ls -ladZ /var/log/faillock/
drwx------. 2 root root unconfined_u:object_r:var_log_t:s0 18 Dec  3 07:38 /var/log/faillock/

With tmpfile:

 # ls -ladZ /var/log/faillock/
drwx------. 2 root root system_u:object_r:faillog_t:s0 18 Dec  3 07:42 /var/log/faillock/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@markusboehme markusboehme Awaiting requested review from markusboehme

@xnox xnox Awaiting requested review from xnox

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

ai/skip-comment Stop AI from commenting on PR approved-to-run A repo member has approved this external contribution bincapz/pass bincapz/pass Bincapz (aka. malcontent) scan didn't detect any CRITICALs on the scanned packages.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@89luca89 @egibs @a-crate @sergiodj @xnox